-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make Methods in Serializers virtual so they can be overwritten in der… #33
Conversation
And this one? is it ok? |
I've been thinking about this, but I'm not too enthusiastic about making the methods virtual. In fact, the classes should be sealed, but somehow I missed adding that. What you're really interested in is the GenerateWriterMethod/GenerateReaderMethod in GenericSerializer, right? We could make public static functions for those, which you could then call from your custom serializer. |
No, im interested in the Handles method of the generic serializer, cause my types don't have the serializable attribute! |
What I mean is that if the methods I mentioned were public, you could easily implement a custom serializer that would call those methods, right? As I said, I'm not enthusiastic about making the classes inheritable. And especially as the only use case so far has been your issue, serializing classes that are not marked as Serializable. That's not a proper use case, as it says that the classes weren't designed to be serialized and thus they really shouldn't be serialized. I'm not saying you shouldn't serialize them, and maybe they work fine for you. But I'm saying that I don't want to change NetSerializer too much to accommodate such a hack use case. |
I understand! But I don't see what the problem could be if you make them virtual? I could also copy the Code of GenericSerializer, but for that a few of the Methods of your serializer need to be public visible! |
The problem with making classes inheritable (and making methods virtual) is that you should design the classes for that use. Just blindly changing methods virtual may easily create problems. Now, could be that the classes here are simple and making them virtual is not a big deal. But again, I'd rather not go that way just to support your problem case. It is ok to make some methods public in the serializer. It should be possible for the user to create custom serializers that do the same work than the built-in serializers. So it should be possible to copy the built-in serializers and have them as custom serializers. I haven't tested custom serializers much, so I'm sure there are private methods that should be made public. |
I was looking a bit at this problem, and I still have no solution. I think it's clear that the user should be able to write custom serializers that would be as good as the current built-in ones. But at the same time I'm not ready to make all the methods and classes public which are currently needed to do that, as then the public API would grow a lot, with rather low level methods which I'd then need to support in later NetSerializer versions. |
No problem, if you won't change, I will create my own Nuget Package from my fork! |
That's probably best for the time being. I'll continue working on this when I have time. I want to keep the public API clean, so this needs some thought on how to implement. I've created an issue for this work: #39 |
Closing this. There's issue #39 for tracking the work needed. |
…ived classes